-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Endpoint] Task/basic endpoint list #55623
Conversation
9d6f576
to
a9b280c
Compare
Add these labels: Add [Endpoint] to the PR title - ex: [Endpoint] Task/basic endpoint list |
@@ -53,15 +54,7 @@ const AppRoot: React.FunctionComponent<RouterProps> = React.memo(({ basename, st | |||
render={() => { | |||
// FIXME: This is temporary. Will be removed in next PR for endpoint list | |||
store.dispatch({ type: 'userEnteredEndpointListPage' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the usePageId
hook instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
export interface EndpointListPagination { | ||
pageIndex: number; | ||
pageSize: number; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move these types to the applications/endpoint/types.ts
file? @paul-tavares suggested this approach and the file was added when we merged the basic alert list PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peluja1012 - Yes, we'll move it (actually mostly delete this, since it was a copy of the server types as a temporary work-around).
I actually opened a separate issue a few days ago (see elastic/endpoint-app-team/issues/131 ) to handle the refactoring items so that @parkiino could continue to focus on getting the list functionality working.
for await (const { action } of actionsAndState()) { | ||
if (action.type === 'userEnteredEndpointListPage') { | ||
const response = await httpPost('/api/endpoint/endpoints'); | ||
for await (const { action, state } of actionsAndState()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paul-tavares @kevinlog @parkiino Are we going to go with the oatSaga
approach or with the simple middleware
approach? We discussed using the simpler approach until we really needed to add complexity. While the alert and endpoint pages can go in different directions, I personally would favor consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, we'll refactor to use Redux middleware. this is also going to be done under elastic/endpoint-app-team/issues/131
@@ -0,0 +1,133 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be moved to the applications/endpoint/view/
directory
hidePerPageOptions: false, | ||
}; | ||
|
||
const onTableChange = ({ page }: { page: { index: number; size: number } }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using useCallback
here
const columns = [ | ||
{ | ||
field: 'host.hostname', | ||
name: 'Host', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should i18n visible names using the i18n package from kibana. Import i18n
from @kbn/i18n
, then use it as:
i18n.translate('xpack.endpoint.management.list', {
defaultMessage: 'Host',
})
<EuiPageContentHeader> | ||
<EuiPageContentHeaderSection> | ||
<EuiTitle> | ||
<h2>Hosts</h2> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to localize Hosts
here there is also a React component you can use -
<FormattedMessage id="xpack.endpoint.managment.list" defaultMessage="Hello World" />
The import path is a little different - import @kbn/i18n/react
@@ -53,15 +54,7 @@ const AppRoot: React.FunctionComponent<RouterProps> = React.memo(({ basename, st | |||
render={() => { | |||
// FIXME: This is temporary. Will be removed in next PR for endpoint list | |||
store.dispatch({ type: 'userEnteredEndpointListPage' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably should follow the same pattern as the Alerts PR use the component={}
attribute fo the Route
and move the usePageId()
call to the Management
Component (we can then delete the userEnteredEndpointListPage
action)
const pageSize = useEndpointListSelector(endpointListPageSize); | ||
const totalItemCount = useEndpointListSelector(endpointTotalHits); | ||
|
||
const paginationSetup = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont
017b998
to
c0fccd6
Compare
x-pack/plugins/endpoint/public/applications/endpoint/view/managing/index.tsx
Outdated
Show resolved
Hide resolved
) { | ||
const pageIndex = endpointListPageIndex(state.endpointList); | ||
const pageSize = endpointListPageSize(state.endpointList); | ||
const response = await httpPost('/api/endpoint/endpoints', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably ok for now, but when we move this to Middleware, we should also handle errors (or throw
's) here. Question will be: what should we do with the errors from the UI standpoint.
}), | ||
}); | ||
// temp: request_page_index to reflect user request page index, not es page index | ||
response.request_page_index = pageIndex; | ||
dispatch({ | ||
type: 'serverReturnedEndpointList', | ||
payload: response, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oatkiller HALP
I heard you were a TS Master Wizard - why are we NOT getting an TS error here? response
is any
(because we did not tell post
what the response from the API looks like). It should have thrown a TS error when trying to assign any
to payload
which has an explicit type on it.
WHYYYYYYY 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can assign any
to anything.
4d6294a
to
5335cc6
Compare
See test cases we need to add for this PR here: https://github.com/elastic/endpoint-app-team/issues/63 |
25e3d35
to
9057c06
Compare
} | ||
|
||
interface UserExitedEndpointListPage { | ||
type: 'userExitedEndpointListPage'; | ||
} | ||
|
||
interface UserPaginatedEndpointListTable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename this for consistency
@@ -4,22 +4,24 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
import { EndpointListData } from './types'; | |||
import { ManagementPagination } from '../../types'; | |||
import { EndpointResultList } from '../../../../../common/types'; | |||
|
|||
interface ServerReturnedEndpointList { | |||
type: 'serverReturnedEndpointList'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename this for consistency
type: 'serverReturnedEndpointList', | ||
payload: response, | ||
}); | ||
dispatch({ type: 'serverReturnedAlertsData', payload: response }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like a bad "copy-and-paste"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should not be there
}; | ||
}; | ||
|
||
export const endpointListReducer: Reducer<EndpointListState, AppAction> = ( | ||
export const endpointListReducer: Reducer<ManagementState, AppAction> = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename this for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parkiino we're going with Management
naming, aren't we? @peluja1012 what would we change here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinlog I think the exported const
is what needs to be renamed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are correct - ignore me
import { useSelector } from 'react-redux'; | ||
import { GlobalState, ManagementState } from '../../types'; | ||
|
||
export function useEndpointListSelector<TSelected>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename this for consistency
import { CreateStructuredSelector } from '../../types'; | ||
|
||
const selector = (createStructuredSelector as CreateStructuredSelector)(selectors); | ||
export const EndpointList = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename this for consistency
const selector = (createStructuredSelector as CreateStructuredSelector)(selectors); | ||
export const EndpointList = () => { | ||
usePageId('managementPage'); | ||
const dispatch = useDispatch<(a: EndpointListAction) => void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename this for consistency
[dispatch] | ||
); | ||
|
||
const columns = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memoize this with useMemo()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok if my comments are addressed in a subsequent PR.
Looks like test is failing due to this https://github.com/elastic/kibana/pull/57506/files |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Add Endpoint plugin and Resolver embeddable (#51994) * Add functional tests for plugins to x-pack (so we can do a functional test of the Resolver embeddable) * Add Endpoint plugin * Add Resolver embeddable * Test that Resolver embeddable can be rendered Conflicts: x-pack/.i18nrc.json x-pack/test/api_integration/apis/index.js * [Endpoint] Register endpoint app (#53527) * register app, create functional test * formatting * update tests * adjust test data for endpoint * add endpoint tests for testing spaces, app enabled, disabled, etc * linting * add read privileges to endpoint * rename variable since its used now * remove deprecated context * remove unused variable * fix type check * correct test suite message Co-Authored-By: Larry Gregory <lgregorydev@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Larry Gregory <lgregorydev@gmail.com> * [Endpoint] add react router to endpoint app (#53808) * add react router to endpoint app * linting * linting * linting * correct tests * change history from hash to browser, add new test util * remove default values in helper functions * fix type check, use FunctionComponent as oppsed to FC * use BrowserRouter component * use BrowserRouter component lin * add comments to test framework, change function name to include browserHistory Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * EMT-issue-65: add endpoint list api (#53861) add endpoint list api * EMT-65:always return accurate endpoint count (#54423) EMT-65:always return accurate endpoint count, independent of paging properties * Resolver component w/ sample data (#53619) Resolver is a map. It shows processes that ran on a computer. The processes are drawn as nodes and lines connect processes with their parents. Resolver is not yet implemented in Kibana. This PR adds a 'map' type UX. The user can click and drag to pan the map and zoom using trackpad pinching (or ctrl and mousewheel.) There is no code providing actual data. Sample data is included. The sample data is used to draw a map. The fundamental info needed is: process names the parent of a process With this info we can topologically lay out the processes. The sample data isn't yet in a realistic format. We'll be fixing that soon. Related issue: elastic/endpoint-app-team#30 * Resolver test plugin not using mount context. (#54933) Mount context was deprecated. Use core.getStartServices() instead. * Resolver nonlinear zoom (#54936) * [Endpoint] add Redux saga Middleware and app Store (#53906) * Added saga library * Initialize endpoint app redux store * Resolver is overflow: hidden to prevent obscured elements from showing up (#55076) * [Endpoint] Fix saga to start only after store is created and stopped on app unmount (#55245) - added `stop()`/`start()` methods to the Saga Middleware creator factory - adjust tests based on changes - changed application `renderApp` to stop sagas when react app is unmounted * Resolver zoom, pan, and center controls (#55221) * Resolver zoom, pan, and center controls * add tests, fix north panning * fix type issue * update west and east panning to behave like google maps * [Endpoint] FIX: Increase tests `sleep` default duration back to 100ms (#55492) Revert `sleep()` default duration, in the saga tests, back to 100ms in order to prevent intermittent failures during CI runs. Fixes #55464 Fixes #55465 * [Endpoint] EMT-65: make endpoint data types common, restructure (#54772) [Endpoint] EMT-65: make endpoint data types common, use schema changes * Basic Functionality Alert List (#55800) * sets up initial grid and data type * data feeds in from backend but doesnt update * sample data feeding in correctly * Fix combineReducers issue by importing Redux type from 'redux' package * Add usePageId hook that fires action when user navigates to page * Strict typing for middleware * addresses comments and uses better types * move types to common/types.ts * Move types to endpoint/types.ts, address PR comments blah 2 Co-authored-by: Pedro Jaramillo <peluja1012@gmail.com> * [Endpoint] Add Endpoint Details route (#55746) * Add Endpoint Details route * add Endpoint Details tests * sacrifices to the Type gods * update to latest endpoint schema Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * [Endpoint] EMT-67: add kql support for endpoint list (#56328) [Endpoint] EMT-67: add kql support for endpoint list * [Endpoint] ERT-82 ERT-83 ERT-84: Alert list API with pagination (#56538) * ERT-82 ERT-83 ERT-84 (partial): Add Alert List API with pagination * Better type safety for alert list API * Add Test to Verify Endpoint App Landing Page (#57129) Conflicts: x-pack/test/functional/page_objects/index.ts * fixes render bug in alert list (#57152) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Resolver: Animate camera, add sidebar (#55590) This PR adds a sidebar navigation. clicking the icons in the nav will focus the camera on the different nodes. There is an animation effect when the camera moves. Conflicts: yarn.lock * [Endpoint] Task/basic endpoint list (#55623) * Adds host management list to endpoint security plugin Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * [Endpoint] Policy List UI route and initial view (#56918) * Initial Policy List view * Add `endpoint/policy` route and displays Policy List * test cases (both unit and functional) Does not yet interact with API (Ingest). * Add ApplicationService app status management (#50223) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. * Implements `getStartServices` on server-side (#55156) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. * [ui/utils/query_string]: Remove unused methods & migrate apps to querystring lib (#56957) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. Co-authored-by: Kevin Logan <56395104+kevinlog@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Larry Gregory <lgregorydev@gmail.com> Co-authored-by: nnamdifrankie <56440728+nnamdifrankie@users.noreply.github.com> Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Co-authored-by: Paul Tavares <56442535+paul-tavares@users.noreply.github.com> Co-authored-by: Pedro Jaramillo <peluja1012@gmail.com> Co-authored-by: Dan Panzarella <pzl@users.noreply.github.com> Co-authored-by: Madison Caldwell <madison.rey.caldwell@gmail.com> Co-authored-by: Charlie Pichette <56399229+charlie-pichette@users.noreply.github.com> Co-authored-by: Candace Park <56409205+parkiino@users.noreply.github.com> Co-authored-by: Pierre Gayvallet <pierre.gayvallet@gmail.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>
Summary
https://github.com/elastic/endpoint-app-team/issues/63
This PR adds a basic UI for the endpoint list page. Although it connects to the endpoints api, data is still being mocked.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Documentation was added for features that require explanation or tutorials- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
Test cases